Skip to content

fix: harden proxy error handling, worktree CLI, and config validation - #73

Open
reopt-eric wants to merge 6 commits into
mainfrom
fix/review-hardening
Open

fix: harden proxy error handling, worktree CLI, and config validation#73
reopt-eric wants to merge 6 commits into
mainfrom
fix/review-hardening

Conversation

@reopt-eric

Copy link
Copy Markdown
Contributor

개요

전체 프로젝트 리뷰에서 발견한 문제들을 영역별로 수정했습니다. 모든 변경에 회귀 테스트를 추가했고, 적대적 리뷰로 새 버그가 없음을 교차 검증했습니다.

변경 내용

🔴 High

  • proxy 에러 핸들러 중복 실행 방지 (server.ts): proxyRes/proxyReq error와 정상 완료가 동시에 clientRes를 건드려 발생하던 ERR_HTTP_HEADERS_SENT 크래시를 settled 플래그로 차단. 클라이언트 중단 시 proxyRes.destroy()로 소켓 누수 방지, hop-by-hop 헤더 스트립(RFC 7230), 에러 페이지 ' 이스케이프.
  • worktree CLI 입력 검증 (worktree.tsx): destroyisValidSubdomain 가드 추가(경로 탈출·hook cwd 차단), git 호출을 execFileSync로 전환(셸 제거), 포트 할당 TOCTOU 재읽기+충돌 감지, { worktrees }만 기록해 레거시 .dev-proxy.json 부활 방지.
  • doctor mjs 검증 복구 (doctor.tsx): worktreeConfig/routes를 resolved 싱글톤에서 읽도록 변경 — mjs 프로젝트(현재 표준)에서 통째로 스킵되던 검증을 복구.

🟠 Medium

  • store 응답 detail 유실 수정 (store.ts): active→idle 전환 후 완료된 요청의 응답 헤더가 영구 누락되던 문제를 detailMap.has(id)면 idle이어도 merge하도록 수정(LRU 한도 유지). eviction 후 필터 캐시 명시적 무효화.
  • worktree 라우팅 검증 (worktrees.ts, config-io.ts): 미등록 service는 null 반환(silent fallback 제거), port 값 검증.

🟡 Low

  • cli.ts: 전역 플래그를 command 위치에서만 처리 → 하위 인자 hijack 방지
  • migrate.tsx: 레거시 파일 삭제 실패를 cleanup-failed로 노출
  • init.tsx: 포트 파싱을 Number로 통일("4000abc" 거부)
  • worktree.tsx: findOwningProject 경로 정규화 + 최장 prefix 우선

테스트

  • 460개 통과 (+13개 신규), typecheck/lint/format 모두 통과

보류

  • app.tsx 마우스 (휠 coalesce / 클릭 stale offset): 렌더 스로틀이 화면-offset 일관성을 유지하고 selectByFilteredIndex가 bounds-check를 하므로 영향이 eviction 시의 드문 edge에 한정. UI는 수동 검증 영역이라 별도 작업으로 분리.

🤖 Generated with Claude Code

reopt-eric and others added 6 commits June 13, 2026 21:19
A single request could surface a terminal event from multiple sources
(proxyRes error, proxyReq error, normal completion), firing a second
writeHead/end and throwing ERR_HTTP_HEADERS_SENT. Guard with a settled
flag so only the first one emits and touches clientRes. Also destroy the
upstream stream when the client aborts mid-response, escape single quotes
in error pages, and strip hop-by-hop headers (RFC 7230) before relaying.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A request for an unregistered worktree service silently fell back to
another service's first port, masking misconfiguration and risking
routing to the wrong app. Return null for a named-but-absent service
(per the no-silent-fallback routing invariant); only default to the
first port when no service is given. Also validate that port values are
finite positive integers so a malformed instance file can't break
routing with an invalid URL.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
worktree destroy skipped the isValidSubdomain guard that create applies,
letting an unvalidated branch flow into {branch} directory substitution
and the post-remove hook cwd (path traversal). Add the guard before any
resolution. Switch git calls to execFileSync to remove the shell, pass
only { worktrees } to writeProjectConfig so a legacy .dev-proxy.json
isn't resurrected, re-read worktrees before writing to narrow the port
TOCTOU window, and prefer the longest matching prefix in findOwningProject.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
doctor read worktreeConfig and routes from readProjectConfig, which only
parses JSON — so for mjs-based projects (now the standard) the value was
always undefined and the entire portRange/services/directory validation
block was silently skipped. Source worktreeConfig and routes from the
resolved ProjectConfig singleton instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a request started while detail collection was active but its
request:complete arrived after the 30s idle timeout, the response
headers were dropped and the detail panel showed (awaiting response)
forever. Collect detail when active OR when the id already holds a detail
entry, so an in-flight request's response is merged regardless of idle
state (still bounded by the LRU). Also invalidate the filtered cache
explicitly after in-place eviction.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Treat --help/--version as global only in the command position so a
subcommand argument or flag isn't hijacked. Surface a cleanup-failed
status when migrate cannot delete the legacy .dev-proxy.json (it was
swallowed silently, leaving config precedence ambiguous). Reject port
input with trailing garbage in init by parsing with Number instead of
parseInt.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.11765% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/proxy/server.ts 91.66% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant